-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rule 6.1.1 Primary constructor should be defined implicitly in the declaration of class #435
Conversation
### What's done: * Refactoring
### What's done: * Refactoring
Codecov Report
@@ Coverage Diff @@
## master #435 +/- ##
============================================
- Coverage 81.83% 81.76% -0.07%
- Complexity 1501 1528 +27
============================================
Files 71 72 +1
Lines 3765 3861 +96
Branches 1209 1234 +25
============================================
+ Hits 3081 3157 +76
Misses 210 210
- Partials 474 494 +20
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
### What's done: * Create init block for other statements * Refactoring
### What's done: * Use KotlinParser
### What's done: * Code style
### What's done: * Code style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some things need clarification
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/SingleConstructorRule.kt
Outdated
Show resolved
Hide resolved
node | ||
.findChildByType(CLASS_BODY) | ||
?.getAllChildrenWithType(SECONDARY_CONSTRUCTOR) | ||
?.singleOrNull() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will you do - if there are no secondaryConstructors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing, as there is nothing to convert,
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/SingleConstructorRule.kt
Outdated
Show resolved
Hide resolved
val classInitializer = kotlinParser.createNode( | ||
""" | ||
|init { | ||
| ${otherStatements.joinToString("\n") { it.text }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started thinking that there can be issues right now:
constructor(a: Int) {
val f = F(a)
this.a = f.foo()
}
how it should work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can't reliably automate complex cases. Your example will be converted to
class Foo(val a: Int) {
init {
f = F(a)
}
}
and it will break the code. In my opinion, correct form of this code should be
class Foo(a: Int) {
val a: Int by lazy {
val f = F(a)
f.foo()
}
}
but there may be other valid cases for refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a Kdoc with a fixme for it
### What's done: * Fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
### What's done: * Added Fixme and disabled test
### What's done: * Added more logic on init block creation
### What's done: * Fixes & smoke test example
# Conflicts: # info/diktat-kotlin-coding-style-guide-en.md
### What's done: * Code style edits applied
### What's done: * Code style
### What's done: * Code style
Which rule and warnings did you add?
This pull request closes #429
Actions checklist
Fixme